Skip to content

fix: rebalance Pierre word highlights in split view#203

Open
benvinegar wants to merge 1 commit intomainfrom
fix/word-diff-highlight-contrast
Open

fix: rebalance Pierre word highlights in split view#203
benvinegar wants to merge 1 commit intomainfrom
fix/word-diff-highlight-contrast

Conversation

@benvinegar
Copy link
Copy Markdown
Member

Summary

  • keep Pierre word-level highlights visible in split view by deriving inline emphasis backgrounds from the diff row colors when theme defaults are too subtle
  • stop the fallback blend at the first visible shade, with finer blend steps so the highlight stays noticeable without looking too bright
  • add model- and render-level tests for inline word-diff emphasis and record the user-visible fix in CHANGELOG.md

Testing

  • bun run typecheck
  • bun test

This PR description was generated by Pi using OpenAI o4-mini

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 14, 2026

Greptile Summary

This PR fixes invisible word-level highlights in Pierre split-view by introducing a wordDiffHighlightBg helper that checks whether the theme's existing addedContentBg/removedContentBg has enough contrast against the row background, and if not, blends incrementally toward the sign color until a minimum Manhattan distance of 28 is reached. It also consolidates the blendHex utility (previously local to renderRows.tsx) into a new shared src/ui/lib/color.ts module alongside the new hexColorDistance helper.

Confidence Score: 5/5

  • This PR is safe to merge — the logic is correct, the type narrowing is sound, and both model-level and render-level tests are in place.
  • No P0 or P1 findings. The strengthening loop correctly returns the first blend that meets the threshold, the StackLineCell["kind"] subset is compatible with the SplitLineCell["kind"]-typed cache, and the module-level cache is bounded to the small fixed set of themes. All remaining observations are P2 style notes.
  • No files require special attention.

Important Files Changed

Filename Overview
src/ui/lib/color.ts New shared color utility module exporting blendHex (moved from renderRows.tsx with input validation added) and the new hexColorDistance (Manhattan RGB distance); both implementations are correct.
src/ui/diff/pierre.ts Adds wordDiffHighlightBg + strengthenWordDiffBg to derive a visible inline word-diff background; replaces the raw theme-color lookup in both makeSplitCell and makeStackCell. Logic and typing are correct — StackLineCell["kind"] is a strict subset of SplitLineCell["kind"] so the shared cache Record is safe.
src/ui/diff/renderRows.tsx Removes the local hexToRgb/blendHex helpers now that they live in color.ts; all call sites updated to import from the shared module. Clean refactor with no behaviour change.
src/ui/diff/pierre.test.ts Updates the model-layer split-view test to assert that word-diff spans carry a background (any, not the exact theme constant) and that non-word spans don't; correctly reflects the new strengthening behaviour.
src/ui/components/ui-components.test.tsx Adds a render-level integration test that measures the actual pixel background distance between word-diff spans and their surrounding spans, asserting ≥ 28 Manhattan units; polling loop correctly waits for async highlighting. Minor: spanIndex <= 0 silently skips the case where the marker lands at span index 0, but the test input ensures "41"/"42" never do.
CHANGELOG.md Appends a user-visible fix entry to the [Unreleased] ### Fixed section as prescribed by the project conventions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["wordDiffHighlightBg(kind, theme)"] --> B{Cache hit\nfor theme.id?}
    B -- Yes --> C["Return cached[kind]"]
    B -- No --> D["Check hexColorDistance(\naddedContentBg, addedBg\n) ≥ 28?"]
    D -- Yes --> E["addition = addedContentBg"]
    D -- No --> F["strengthenWordDiffBg(\naddedBg, addedSignColor\n)"]
    F --> G["Loop: blend ratio 0.005→0.2\nstep 0.005"]
    G --> H{distance from\nlineBg ≥ 28?}
    H -- Yes --> I["Return candidate\n(early exit)"]
    H -- No, more steps --> G
    H -- No, loop done --> J["Return max-blend\ncandidate (0.2)"]
    E --> K["Build cache record\naddition / deletion / context / empty"]
    F --> K
    J --> K
    I --> K
    K --> L["wordDiffBackgroundCache\n.set(theme.id, cached)"]
    L --> C
Loading

Reviews (1): Last reviewed commit: "fix(diff): rebalance split-view word hig..." | Re-trigger Greptile

@benvinegar
Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant